Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.0][FIX][report_xls] Protect xlwt import. #62

Merged
merged 2 commits into from
Jul 25, 2016

Conversation

yajo
Copy link
Member

@yajo yajo commented Jul 25, 2016

Given xlwt could happen to not be installed because it is an optional dependency.

See odoo/odoo#12897 (comment).

@Tecnativa

Given `xlwt` could happen to not be installed because it is an optional dependency.

See odoo/odoo#12897 (comment).
@hbrunn
Copy link
Member

hbrunn commented Jul 25, 2016

👍

2 similar comments
@pedrobaeza
Copy link
Member

👍

@carlosdauden
Copy link

👍

@pedrobaeza
Copy link
Member

Please correct Travis now that the module has changed. In concrete, the line https://travis-ci.org/OCA/reporting-engine/jobs/147163836#L459. You should bypass the error with # pylint: disable=W0123 in the line, because eval is needed by some of the modules as I remember.

@pedrobaeza pedrobaeza merged commit 2895aef into OCA:8.0 Jul 25, 2016
@pedrobaeza pedrobaeza deleted the 8.0-report_xls-protected_import branch July 25, 2016 15:02
@sergiocorato
Copy link

Hi,
after this merge this import
from openerp.addons.report_xls.report_xls import report_xls
doesn't fail?

@pedrobaeza
Copy link
Member

What are you talking about?

@sergiocorato
Copy link

sergiocorato commented Jul 26, 2016

@pedrobaeza
Copy link
Member

That's something totally different (in another repo). That should be protected too.

@sergiocorato
Copy link

I refer to report_xls class name, renamed to ReportXls, which is not more recognized

@pedrobaeza
Copy link
Member

OK, please fill a PR with that change.

@sergiocorato
Copy link

#63

@StefanRijnhart
Copy link
Member

More breakage in account-financial-tools. OCA/account-financial-tools#392

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants